feat(workspace): add workspace resource model with scoping, membership, and isolation (review mirror)#14
Open
rhuss wants to merge 14 commits into
Open
feat(workspace): add workspace resource model with scoping, membership, and isolation (review mirror)#14rhuss wants to merge 14 commits into
rhuss wants to merge 14 commits into
Conversation
|
Important Review skippedToo many files! This PR contains 177 files, which is 127 over the limit of 50. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (177)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Owner
Author
|
@devin-ai-integration please review |
Thread workspace through compute drivers (Docker, Podman, K8s) with
converged container naming (openshell-{workspace}--{name}-{id}),
workspace labels, and label-based lookup. Fix sandbox-side policy sync
to use learned workspace instead of hardcoding empty string. Fix TUI
all-workspaces view to use per-row workspace for sandbox actions. Add
provider profile to workspace deletion blocking check. Thread workspace
through SSH config generation with workspace-qualified host aliases.
Signed-off-by: Derek Carr <decarr@redhat.com>
Add workspace as a required parameter for resource-scoped operations (create, get, delete, wait_ready, wait_deleted) and optional for list operations that support all_workspaces global lookup. Rename ClusterInferenceConfig to InferenceRouteConfig and set_cluster/get_cluster to set_route/get_route to match the renamed proto RPCs. Signed-off-by: Derek Carr <decarr@redhat.com>
- Backfill provider_credential_refresh_state in workspace migration - Set requires_workspace() to true for refresh state - Reject consecutive hyphens in workspace and sandbox names - Restore parse_host rejection of consecutive hyphens in all segments - Pass workspace from SSH session metadata in put_if call - Require explicit workspace in Python SDK Sandbox() constructor - Add sandbox name character validation matching workspace rules Signed-off-by: Derek Carr <decarr@redhat.com>
- Add watch channel to broadcast workspace from policy poll loop to denial and activity flush tasks, fixing proposals targeting the wrong workspace for non-default sandboxes - Add set_workspace() to CachedOpenShellClient for pre-seeding workspace on fresh connections - Use selected_provider_workspace() in TUI provider get/delete actions instead of current_workspace, fixing cross-workspace misrouting Signed-off-by: Derek Carr <decarr@redhat.com>
Signed-off-by: Derek Carr <decarr@redhat.com>
…le scope binding Provider profiles can be global (platform-scoped) or workspace-scoped. Previously, profile lookups used the ambient request workspace, which caused pre-existing global profiles (workspace="") to become invisible after the workspace migration. Add a `profile_workspace` field to the Provider proto that explicitly declares where the provider's type profile is stored. The field must be empty (global) or match the provider's own workspace — cross-workspace references are rejected. All profile lookup call sites now use `provider.profile_workspace` instead of the ambient workspace. Existing serialized providers get `profile_workspace=""` via protobuf defaults, correctly pointing to their global profiles with no migration. CLI gains `--global-profile` flag on `provider create`. Signed-off-by: Derek Carr <decarr@redhat.com>
…-back handle_revoke_ssh_session passed empty string for workspace in put_if instead of session.object_workspace(), dropping the workspace on the revoked session record. Signed-off-by: Derek Carr <decarr@redhat.com>
The lint handler was passing an empty string to profile_conflict_diagnostics instead of using the request workspace, so it only checked for conflicts in global scope rather than the workspace the user specified. Signed-off-by: Derek Carr <decarr@redhat.com>
- Fix proto comments: ListProviderProfilesRequest.workspace no longer claims two-tier merged resolution, LintProviderProfilesRequest.workspace now documents that it is used for conflict detection - Converge Podman driver labels to openshell.ai/ prefix by importing from driver_utils, matching Docker and Kubernetes drivers - Replace debug_assert with runtime PersistenceError for workspace enforcement in put_message, put_scoped_message, and cas_update_message - Add comment in parse_host explaining -- delimiter safety invariant - Read OPENSHELL_WORKSPACE env var in tab completers instead of defaulting to empty string - Remove unused _workspace params from provider_refresh_defaults and dynamic_token_grant_bindings_for_provider - Remove scattered workspace "default" fallbacks in service_routing, inference, and compute; normalization is handled by resolve_workspace at the handler layer Signed-off-by: Derek Carr <decarr@redhat.com>
- Backfill policy and draft_chunk object types in migration - Add label selector support to all_workspaces sandbox listing - Replace per-member deletion loop with batch delete_all_in_workspace - Extract shared validate_dns1123_label helper from duplicated validation - Add count_in_workspace for efficient member cap enforcement - Document intentional omission of workspace filter in list_by_scope Signed-off-by: Derek Carr <decarr@redhat.com>
Fix workspace-scoped cleanup bugs: settings deletion used empty workspace instead of sandbox workspace, migration backfill missed sandbox_settings, SSH session cleanup scanned globally instead of by workspace, and workspace deletion did not check for blocking sandbox_settings records. Add K8s resource name length validation to reject names exceeding DNS-1123 63-char limit. Update RFC 0011 to match Phase 1 implementation: document inference RPC rename, UpdateProviderProfiles with optimistic concurrency, Provider profile_workspace field, Python SDK workspace requirements, service hostname breaking change, and persistence validation behavior. Signed-off-by: Derek Carr <decarr@redhat.com>
Fix bugs found during PR NVIDIA#2243 review: correct migration type strings, resolve platform-scoped profile workspace handling, fix sandbox workspace_tx single-fire, add missing workspace deletion blockers, reset TUI row indices on workspace switch, fix TUI provider update workspace, and update e2e Python tests for workspace and inference route API changes. Add Phase 2 TODO comments for authorization gaps. Fix pre-existing clippy errors (too_many_arguments, cast_sign_loss, iter_on_single_items). Signed-off-by: Derek Carr <decarr@redhat.com>
…ace args Add Rust handler test and Python e2e test verifying that platform-scoped and workspace-scoped provider profiles are isolated from each other. Fix missing workspace keyword arguments in test_sandbox_api.py and test_sandbox_providers.py that caused TypeErrors. Signed-off-by: Derek Carr <decarr@redhat.com>
Fix reconciler test that stored sandbox settings with workspace "" while the sandbox itself used "default", causing the cleanup delete to miss the record. Fix e2e tests missing the required workspace keyword argument on SandboxClient.create, .get, .delete, and .wait_deleted calls. Signed-off-by: Derek Carr <decarr@redhat.com>
Owner
Author
|
@devin-ai-integration please review |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Mirror of NVIDIA/OpenShell#2243 for bot review analysis (CodeRabbit, Devin).
Original author: @derekwaynecarr
This PR is not intended to be merged. It exists solely to trigger automated review bots on the fork where they are installed.